Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove fabric queue counters. #2862

Merged
merged 8 commits into from
Aug 18, 2023
Merged

Remove fabric queue counters. #2862

merged 8 commits into from
Aug 18, 2023

Conversation

jfeng-arista
Copy link
Contributor

What I did
Remove fabric queue counters.

Why I did it
This is fixing issue #15586. Where the following messages are seen

NOTICE syncd0#syncd: :- setQueueCounterList: FABRIC_QUEUE_STAT_COUNTER: queue oid:0x18d051500000001 does not has supported counters

Based on case CS00012300922, the support of these queue counters objects are removed after 9.x for DNX.

How I verified it

Manually tested an image with the fix on a system that has J2c+ linecards, and do not see the error messages.

Details if related

@prsunny prsunny requested a review from abdosi July 18, 2023 20:27
@jarias-lfx
Copy link

/easycla

@kenneth-arista
Copy link
Contributor

Fixes: sonic-net/sonic-buildimage#15586

@jfeng-arista jfeng-arista mentioned this pull request Jul 31, 2023
Merged
@jfeng-arista
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kenneth-arista
Copy link
Contributor

test_pbh.py::TestPbhBasicEditFlows::test_PbhRuleUpdate failure is unrelated to this PR.

@rlhui rlhui added the P0 label Aug 9, 2023
@jfeng-arista
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfeng-arista
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abdosi
Copy link
Contributor

abdosi commented Aug 17, 2023

what is the functional implication of this change ?

@jfeng-arista
Copy link
Contributor Author

what is the functional implication of this change ? -- the show fabric counters queue command will display 0 values in some field.

@rlhui rlhui merged commit b4fcfc9 into sonic-net:master Aug 18, 2023
15 checks passed
@rlhui
Copy link
Contributor

rlhui commented Aug 18, 2023

@arlakshm , would you please help create an MSFT ADO for this, thanks

@gechiang
Copy link
Contributor

@rlhui , @arlakshm ,
In the interest of time I have created the needed MSFT ADO: 24917858

@yxieca , @StormLiangMS , Please help cherry-pick backport this fix to corresponding release branch, Thanks!

@gechiang
Copy link
Contributor

@arlakshm , @abdosi , @tjchadaga , @jfeng-arista
Since this issue is only present on SAI 9.x and above, 202205 SAI is at SAI 7.x So this PR probably is not needed for 202205 release and only needed for Master, 202305 and later releases, correct?

@gechiang
Copy link
Contributor

@arlakshm , @abdosi , @tjchadaga , @jfeng-arista Since this issue is only present on SAI 9.x and above, 202205 SAI is at SAI 7.x So this PR probably is not needed for 202205 release and only needed for Master, 202305 and later releases, correct?

Confirmed with Arvind over the phone and saw in the issue mentioned that this is required for 202205 because when BRCM removed the support in 9.X they also went back and removed it from 7.X so we do see this same issue on 202205 and require this fix be backported to 202205.
I will also go ahead add this for 202211 and 202305 as well...

@jfeng-arista
Copy link
Contributor Author

@arlakshm , @abdosi , @tjchadaga , @jfeng-arista Since this issue is only present on SAI 9.x and above, 202205 SAI is at SAI 7.x So this PR probably is not needed for 202205 release and only needed for Master, 202305 and later releases, correct?

I remember to see the issue message on 202205 image and I think we may need this into 202205

@gechiang
Copy link
Contributor

@arlakshm , @abdosi , @tjchadaga , @jfeng-arista Since this issue is only present on SAI 9.x and above, 202205 SAI is at SAI 7.x So this PR probably is not needed for 202205 release and only needed for Master, 202305 and later releases, correct?

I remember to see the issue message on 202205 image and I think we may need this into 202205

Yes. I have looked into the original issue filed for this and it is stated clearly there that when BRCM removed it from the latest, they also removed the change from SAI 7.x that is being used by 202205 so this bug fux is needed for 202205 branch.

yxieca pushed a commit that referenced this pull request Aug 30, 2023
@jfeng-arista @yxieca
Remove fabric queue counters. (#2862)
d787d50 
What I did
Remove fabric queue counters.

Why I did it
This is fixing issue #15586. Where the following messages are seen

NOTICE syncd0#syncd: :- setQueueCounterList: FABRIC_QUEUE_STAT_COUNTER: queue oid:0x18d051500000001 does not has supported counters

Based on case CS00012300922, the support of these queue counters objects are removed after 9.x for DNX.
@StormLiangMS
Copy link
Contributor

@jfeng-arista 202305 conflict, could you help to file separate PR for 202305?

@gechiang for vis.

@jfeng-arista
Copy link
Contributor Author

@jfeng-arista 202305 conflict, could you help to file separate PR for 202305?

@gechiang for vis.

ok, please let me know the repo, I can raise a separate one

@gechiang
Copy link
Contributor

gechiang commented Sep 4, 2023

@jfeng-arista 202305 conflict, could you help to file separate PR for 202305?
@gechiang for vis.

ok, please let me know the repo, I can raise a separate one

https://github.com/sonic-net/sonic-swss/tree/202305

@gechiang
Copy link
Contributor

gechiang commented Sep 6, 2023

@jfeng-arista Just a reminder to raise PRs to both 202305 and 202211 branches. Thanks!

@jfeng-arista
Copy link
Contributor Author

@jfeng-arista Just a reminder to raise PRs to both 202305 and 202211 branches. Thanks!

sure , will do

@jfeng-arista
Copy link
Contributor Author

sorry about the late, in 202305 and 202211 ,orchagent does not have this enabled :
orchDaemon->setFabricQueueStatEnabled(false);
, so no need for this PR.

@gechiang
Copy link
Contributor

sorry about the late, in 202305 and 202211 ,orchagent does not have this enabled : orchDaemon->setFabricQueueStatEnabled(false); , so no need for this PR.

Thanks for the clarification on the backport branch.
I have removed the request for the 202305 and 202211 back port.

@jfeng-arista
Copy link
Contributor Author

sorry about the late, in 202305 and 202211 ,orchagent does not have this enabled : orchDaemon->setFabricQueueStatEnabled(false); , so no need for this PR.

Thanks for the clarification on the backport branch. I have removed the request for the 202305 and 202211 back port.

thank you

@arlakshm arlakshm mentioned this pull request Sep 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arlakshm arlakshm arlakshm approved these changes

@prsunny prsunny Awaiting requested review from prsunny prsunny is a code owner

@abdosi abdosi Awaiting requested review from abdosi

Assignees

@jfeng-arista jfeng-arista

Labels
Cherry Pick Conflict_202211 Cherry Pick Conflict_202305 Included in 202205 Branch P0 Request for 202205 Branch
Projects
SONiC Chassis
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.